Stabilize telemetry consent and promote telemetry to production - #706

src/ffi/mxc_ffi/src/lib.rs:464

• This eprintln! is outside the catch_unwind. If persisting consent fails while the embedding host has a closed/broken stderr pipe, the diagnostic write can panic across the extern "C" boundary and abort the host instead of returning MXC_STATUS_CONSENT_WRITE_FAILED. Emit this diagnostic with a fallible write whose error is ignored.

sdk/dotnet/Microsoft.Mxc.Sdk/MxcTelemetry.cs:264

• An unknown native policy string is silently collapsed to Blocked here. That is fail-closed, but it hides the mismatched/corrupt native response despite this PR's requirement to report swallowed failures once. Make parsing expose recognition failure and invoke ReportFailClosed before returning Blocked.

                    var value = outUtf8 is null ? null : Marshal.PtrToStringUTF8((IntPtr)outUtf8);
                    return ParsePolicyState(value);

src/ffi/mxc_ffi/src/lib.rs:719

• The telemetry tests only set this debug-only override; they never prove that the native code honors it. On Windows, cargo test -p mxc_ffi --release compiles the override lookup out, after which the tests at lines 769, 795, and 821 write to the developer's real %LOCALAPPDATA% consent record. Gate the writing tests to debug builds or add the same read-only two-state verification used by the C# fixture/smoke test and abort before any native write.
  src/core/wxc_common/src/telemetry/mod.rs:177
• This changes enabled: None into an enabled state once consent and policy permit it, but the wire contract still says omission is disabled (wire.rs:571-573, also emitted into the dev schema), and models.rs:730-732 says the same. Thus experimental.telemetry: {} can now collect contrary to the published schema. Update the wire/domain documentation and regenerate the dev schema so the config contract matches this intended kill-switch behavior.
  sdk/dotnet/Microsoft.Mxc.Sdk/Native/NativeLibraryResolver.cs:59
• This logging call can itself throw (for example after a host replaces or closes Console.Error). Because Initialize runs in a static constructor, that exception permanently poisons MxcTelemetry, defeating the documented never-throw behavior that this catch is meant to preserve. Make this best-effort with a nested try/catch, as ReportFailClosed already does.

            Console.Error.WriteLine(
                $"mxc: could not register the native library resolver ({ex.GetType().Name}: {ex.Message}). " +
                "Falling back to the default loader.");

sdk/dotnet/Microsoft.Mxc.Sdk/MxcTelemetry.cs:41

• The summary says this method “Always succeeds,” but the implementation deliberately throws MxcException for a genuine non-success native status and wraps unexpected marshalling failures. Document that distinction here so callers do not rely on a false no-throw contract.

    /// <summary>
    /// Read the persisted telemetry consent state. Always succeeds — never
    /// throws for "no decision yet" or "not on Windows"; both are ordinary
    /// return values (<see cref="TelemetryConsentState.Undetermined"/> and
    /// <see cref="TelemetryConsentState.NotApplicable"/> respectively).

sdk/dotnet/Microsoft.Mxc.Sdk/MxcTelemetry.cs:72

• ParseConsentState silently maps any unknown or malformed native string to Undetermined, so a mismatched/corrupt binding is indistinguishable from a genuine undecided user and no deduplicated diagnostic is emitted. Have parsing report whether the value was recognized, then call ReportFailClosed before returning the safe fallback.

This issue also appears on line 263 of the same file.

                    var value = outUtf8 is null ? null : Marshal.PtrToStringUTF8((IntPtr)outUtf8);
                    return ParseConsentState(value);

src/ffi/mxc_ffi/Cargo.toml:17

• This new production dependency bypasses the public mxc-sdk facade even though mxc-sdk::telemetry exposes the same operations. The rest of this crate consistently wraps mxc_sdk (lib.rs:54, streaming.rs:57, state_aware.rs:25), and mxc-sdk explicitly owns facade types so consumers remain decoupled from wxc_common. Route production calls through mxc_sdk::telemetry; keep the feature-enabled wxc_common entry only as a dev-dependency for the test redirector.
  src/core/wxc_common/src/telemetry/policy.rs:177
• This non-NotFound registry failure is collapsed to Unreadable and then Blocked with no diagnostic. That keeps collection off, but an ACL failure becomes indistinguishable from an intentional block and contradicts the documented “every swallowed failure is reported once” behavior; bindings only receive the valid string blocked, so they cannot report it later. Emit a deduplicated, non-throwing diagnostic here while preserving the fail-closed state (and do the same for value-read failures below).

docs/telemetry/telemetry-consent-design.md:375

• This section says the Rust SDK is a pure re-export of wxc_common types, but the implementation deliberately defines crate-owned ConsentState and PolicyState facades. Update the description and example so maintainers do not bypass the SDK boundary based on stale architecture documentation.

`mxc-sdk` — the public Rust SDK — re-exports the consent and policy API
verbatim from `wxc_common::telemetry`, so a Rust consumer gets the same
operations as a Node or C# consumer:

```rust
pub mod telemetry {
    pub use wxc_common::telemetry::consent::{
        get_consent, needs_consent_prompt, set_consent, ConsentState,

src/core/wxc_common/src/telemetry/consent.rs:199

• Caching Option<PathBuf> permanently caches a transient resolution failure. If the token/registry/Shell call fails once in a long-lived SDK host, every later consent read and write remains unavailable until process restart even after the system recovers. Cache only successful paths and retry after None.
  src/core/wxc_common/src/telemetry/consent.rs:220
• SHGetKnownFolderPath requires a non-null token to have TOKEN_QUERY | TOKEN_IMPERSONATE, but this handle requests only TOKEN_QUERY. On systems enforcing that documented contract, LocalAppData resolution fails, so consent always reads as Undetermined and writes fail. Open the process token with the required rights.
  src/core/wxc_common/src/telemetry/consent.rs:358
• These read and parse failures are silently collapsed into Undetermined. That contradicts the PR's guarantee that swallowed fail-closed failures are reported once, and downstream SDKs cannot distinguish this from a genuinely fresh store, so no later layer can report it. Add a deduplicated, non-panicking diagnostic for non-NotFound I/O and parse failures.
  src/core/wxc_common/src/telemetry/policy.rs:159
• PolicyValue::Unreadable discards the registry error and silently returns Blocked. This violates the stated once-per-process reporting guarantee, and bindings see an ordinary blocked policy so they cannot diagnose the failure later. Preserve and report the underlying open/read error through a deduplicated, non-panicking diagnostic path.
  src/core/wxc_common/src/telemetry/consent_cli.rs:119
• --telemetry-consent-source is ignored unless another consent action is present: source-only execution returns None and can continue into a normal sandbox run, while status+source silently discards the value. Since this option is documented only as provenance for grant/revoke, reject it with EX_USAGE when neither write action is set.
  src/core/wxc/src/main.rs:836
• This is not the cheapest fast path: recover_orphaned_state() already ran above. Because the Node SDK invokes this CLI for each consent query, a read-only status call can unexpectedly mutate DACL state and can exceed the SDK's 5-second timeout when recovery is slow, yielding a false fail-closed result. Move consent handling immediately after CLI parsing, before force-reclaim and DACL recovery.

    // --telemetry-consent-{status,grant,revoke}: administer the persisted
    // consent flag and exit. Run before --probe (cheapest possible fast
    // path — no config parsing, no policy defaults needed).
    if handle_telemetry_consent_flags(&cli) {

src/core/mxc-sdk/src/telemetry.rs:188

• This new public Rust SDK operation exposes Result<(), String>, unlike the SDK's typed Error/ErrorCode facade (src/core/mxc_engine/src/error.rs:72-88). Consumers cannot reliably match consent-write failures, and changing this after release would break the API. Return an SDK-owned error type or add a stable consent-write error variant before shipping.

/// Record the user's decision. Returns `Err` on a non-Windows host, and on
/// Windows if the decision could not be persisted — a caller must not treat a
/// failed write as consent.
pub fn set_consent(granted: bool, source: &str) -> Result<(), String> {
    inner_consent::set_consent(granted, source)

src/ffi/mxc_ffi/Cargo.toml:17

• The runtime dependency bypasses the repository's stated boundary: mxc_ffi is a C ABI over mxc_sdk (src/ffi/mxc_ffi/src/lib.rs:4), and this PR already adds crate-owned telemetry facades in mxc_sdk. Route the four exports through mxc_sdk::telemetry and keep wxc_common only as a dev-dependency for the test guard; otherwise the FFI layer is coupled directly to foundation internals.
  scripts/check-telemetry-policy-parity.js:89
• This check rejects the implementation in this PR: ParsePolicyState uses _ => UnrecognizedPolicyState(value), not _ => TelemetryPolicyState.Blocked, so csharpDefault is always false and the supposedly passing parity script exits with an error. Accept the helper form only after verifying that helper returns Blocked.

const csharpDefault = /_\s*=>\s*TelemetryPolicyState\.Blocked/.test(
  parseBody[1]
);
if (!csharpDefault) {
  errors.push(
    "C# ParsePolicyState must fail closed with `_ => TelemetryPolicyState.Blocked`"
  );
}
// The default arm covers "blocked", so treat it as handled.
csharpStates.add("blocked");

docs/telemetry/telemetry-consent-design.md:139

• The formula says an omitted enabled field permits collection (None != Some(false)), but the implementation and surrounding text require an explicit Some(true). Correct the formula so this privacy gate is documented unambiguously.

This issue also appears on line 368 of the same file.

         && request.experimental.telemetry.enabled != Some(false)

docs/telemetry/telemetry-consent-design.md:419

• The implementation checklist repeats the obsolete “pure re-export” architecture even though the SDK now owns facade enums in src/core/mxc-sdk/src/telemetry.rs. Update this row to match the implemented boundary.

| `src/core/mxc-sdk/src/lib.rs` | `pub mod telemetry` re-exporting `get_consent` / `set_consent` / `needs_consent_prompt` / `ConsentState` from `wxc_common`, so the public **Rust** SDK offers the same consent surface as the Node and C# SDKs. Pure re-export — no Rust-SDK-specific logic. | ✅ Done |

scripts/check-telemetry-policy-parity.js:89

• This check currently fails against the C# implementation it is meant to validate: ParsePolicyState delegates its default arm to UnrecognizedPolicyState(value), while this regex only accepts a literal TelemetryPolicyState.Blocked. The script therefore reports an error even though that helper returns Blocked. Validate both the delegating arm and its fail-closed helper (while retaining the explicit "blocked" state check).

const csharpDefault = /_\s*=>\s*TelemetryPolicyState\.Blocked/.test(
  parseBody[1]
);

src/core/wxc_common/src/telemetry/consent.rs:373

• These branches silently collapse both an unreadable store and malformed JSON into an ordinary Undetermined result. Because the FFI/SDK receives a successful state, no outer reporter can diagnose the failure. This contradicts the PR's guarantee that swallowed failures are reported once per process (and the design document's statement that corrupt records are logged). Keep a missing file quiet, but report and deduplicate other read/parse failures before failing closed.
  src/core/wxc_common/src/telemetry/policy.rs:178
• This discarded error (and the analogous get_value error below) makes an unreadable policy indistinguishable from a legitimate administrative block to every outer surface, so the failure is never reported. The PR promises that fail-closed failures are diagnosed once per process; preserve Blocked, but pass the error to a deduplicated, non-throwing reporter.
  src/core/wxc_common/src/telemetry/consent_cli.rs:119
• --telemetry-consent-source by itself is accepted by clap but reaches this branch and returns None, so the executor continues into normal config parsing and can even run a sandbox if config was also supplied. Since this option is documented only as provenance for grant/revoke, treat the source-only form as EX_USAGE instead of silently ignoring it.
  src/core/mxc-sdk/src/telemetry.rs:189
• This new public SDK operation exposes a raw String error, unlike the crate's other public fallible operations, which return the typed Error/ErrorCode facade (src/core/mxc-sdk/src/lib.rs:100,118,136,146). Callers cannot reliably distinguish a consent persistence failure without parsing text, even though the C# surface already exposes ConsentWriteFailed. Return a crate-owned typed error (or a dedicated consent error) and map the internal message into it.

/// Record the user's decision. Returns `Err` on a non-Windows host, and on
/// Windows if the decision could not be persisted — a caller must not treat a
/// failed write as consent.
pub fn set_consent(granted: bool, source: &str) -> Result<(), String> {
    inner_consent::set_consent(granted, source)
}

docs/telemetry/telemetry-consent-design.md:385

• This section still describes the Rust SDK as a pure re-export, but the current implementation deliberately added SDK-owned ConsentState/PolicyState facades in src/core/mxc-sdk/src/telemetry.rs. Update the design record so it no longer documents the architecture that the earlier review fix removed.

This issue also appears on line 419 of the same file.

This is a **pure re-export** — there is deliberately no Rust-SDK-specific
consent logic to keep in sync. A Rust host calls `needs_consent_prompt()`
at first sandbox run, shows its own UI, then calls `set_consent(..)`, and
can call `get_consent()`/`set_consent(..)` from a settings surface later —
exactly the flow described in §8.

src/ffi/mxc_ffi/Cargo.toml:17

• The repository architecture defines mxc_ffi as a C ABI over mxc-sdk, but this production dependency lets the new exports bypass the SDK telemetry facade and call the foundation crate directly. Use mxc_sdk::telemetry in the FFI exports and keep wxc_common only as a dev-dependency for the test redirector; this preserves the established layer boundary documented in .github/copilot-instructions.md.
  docs/telemetry/telemetry-consent-design.md:429
• This checklist says three entry points were added, but the script and FFI surface now require four: get consent, set consent, needs prompt, and get policy. Correct the count so the design record accurately describes the codegen gate.

| `scripts/check-dotnet-bindings-codegen.js` | Extended `REQUIRED_ENTRY_POINTS` with the three new FFI exports. | ✅ Done |

src/core/wxc/src/main.rs:838

• The consent handler is not actually the first fast path here: recover_orphaned_state() runs beforehand and can modify filesystem ACLs and emit unrelated stderr. Thus a Node/C# “read-only” consent status query performs DACL recovery, contrary to the design document's guarantee that these flags run before any other startup work. Move this handler ahead of recovery, or explicitly document and test the side effect if it is intentional.

    // --telemetry-consent-{status,grant,revoke}: administer the persisted
    // consent flag and exit. Run before --probe (cheapest possible fast
    // path — no config parsing, no policy defaults needed).
    if handle_telemetry_consent_flags(&cli) {
        return;
    }

sdk/dotnet/Microsoft.Mxc.Sdk/Microsoft.Mxc.Sdk.csproj:75

• This duplicates the identical InternalsVisibleTo item already present at lines 38–40, causing the SDK to generate the same friend-assembly attribute twice. Keep only one item group.

  <ItemGroup>
    <InternalsVisibleTo Include="Microsoft.Mxc.Sdk.Tests" />
  </ItemGroup>

src/ffi/mxc_ffi/Cargo.toml:17

• This adds a production dependency from mxc_ffi directly to the foundation crate even though the new mxc_sdk::telemetry facade exposes all four operations. That bypasses the established FFI → mxc-sdk boundary (.github/copilot-instructions.md:211) and defeats the facade introduced in src/core/mxc-sdk/src/telemetry.rs:11-19. Call the SDK facade from the FFI entry points and keep wxc_common only as the dev-dependency needed by the test harness.
  src/core/wxc/src/main.rs:140
• source is documented and typed as free-form, but clap will treat a hyphen-leading value as another option by default. For example, setTelemetryConsent(true, "--telemetry-consent-revoke") becomes grant + revoke and exits with the mutual-exclusion error instead of persisting that source. Allow hyphen values (or explicitly narrow/validate the advertised source contract).

    #[arg(long = "telemetry-consent-source")]
    telemetry_consent_source: Option<String>,

sdk/dotnet/Microsoft.Mxc.Sdk/Microsoft.Mxc.Sdk.csproj:75

• This duplicates the existing InternalsVisibleTo item at lines 38–40, causing the same assembly attribute to be generated twice. Keep a single declaration.

  <ItemGroup>
    <InternalsVisibleTo Include="Microsoft.Mxc.Sdk.Tests" />
  </ItemGroup>

src/core/mxc-sdk/src/telemetry.rs:188

• This is the only fallible public mxc-sdk operation that returns an unstructured String; the rest of the SDK returns its crate-owned Error/ErrorCode. Consumers cannot reliably distinguish a non-Windows refusal from persistence failure without parsing text, and the error wording becomes part of the API contract. Return a typed SDK-owned consent error (or add ConsentWriteFailed to the existing facade) and map the internal string into it.

/// Record the user's decision. Returns `Err` on a non-Windows host, and on
/// Windows if the decision could not be persisted — a caller must not treat a
/// failed write as consent.
pub fn set_consent(granted: bool, source: &str) -> Result<(), String> {
    inner_consent::set_consent(granted, source)

scripts/check-telemetry-policy-parity.js:116

• The parity check validates only the exported TypeScript union, not isPolicyState, which is the actual runtime parser (sdk/node/src/telemetry.ts:90). Adding a Rust state and updating the union while forgetting that guard would pass this check but still make Node reject the state as blocked. Compare the guard’s accepted literals with the union too.

// --- TypeScript: the exported union ---------------------------------------
const tsSrc = readFileSync(tsPath, "utf8");
const unionMatch = tsSrc.match(
  /export type TelemetryPolicyState\s*=\s*([^;]+);/

sdk/node/src/telemetry.ts:166

• A missing or non-boolean needsPrompt is silently coerced to false. Because queryTelemetryConsent() only reports failures when error is set, an outdated or malformed native payload becomes indistinguishable from a genuine “do not prompt” result and violates the documented once-per-failure diagnostic behavior. Validate this field and return a fail-closed result with error so the existing reporter runs; update the query/error documentation and test accordingly.

    needsPrompt: record.needsPrompt === true && policy !== 'blocked',

src/core/wxc_common/src/telemetry/consent_cli.rs:213

• The stated reason for pinning JSON field order is no longer true: the smoke test parses output with ConvertFrom-Json and compares properties. Keeping this test unnecessarily makes harmless field reordering a failure; remove it unless field order is an explicit CLI contract.
  sdk/dotnet/Microsoft.Mxc.Sdk/Microsoft.Mxc.Sdk.csproj:75
• InternalsVisibleTo is already declared at lines 38–40, so this second item emits duplicate friend-assembly metadata. Remove the duplicate block.

  <ItemGroup>
    <InternalsVisibleTo Include="Microsoft.Mxc.Sdk.Tests" />
  </ItemGroup>

src/core/mxc_engine/src/lib.rs:218

• Dropping a live streaming handle only unregisters telemetry; it emits no terminal event. This is the normal path for mxc_sandbox_free/C# Dispose, which kills a still-running child during drop, so those cancelled executions disappear from telemetry despite this wrapper's exactly-one-terminal-event contract. Emit a per-invocation cancellation/failure event before releasing the provider reference.
  src/core/mxc_engine/src/lib.rs:195
• A timed-out state-aware exec is converted to MxcError::backend_error here, so emit_sdk_state_aware classifies it as ProcessError. The timeout was already identified above via ErrorKind::TimedOut, and one-shot telemetry reports it as Timeout; preserve that distinction for state-aware telemetry as well.
  sdk/node/src/types.ts:284
• The public comment still says true “force[s] on” telemetry, but the promoted switch is only a per-invocation request and cannot bypass persisted consent or administrative policy. This wording contradicts the new privacy contract and may lead SDK consumers to believe this flag is sufficient by itself.
  src/core/mxc-sdk/src/telemetry.rs:41
• This example equates persisted consent with “telemetry on,” but collection can still be blocked by administrative policy or omitted per-run enablement. Since get_consent() reports only the user's decision, label these branches as consent state rather than effective telemetry state.

docs/telemetry/telemetry-policy.md:142

• This is not the response the implementation emits. Policy is reported as a separate ceiling and does not rewrite consent's effective state: the smoke test explicitly expects stored/effective granted while policy is blocked (tests/scripts/run_telemetry_consent_smoke_test.ps1:149-151). The actual status has effectiveState:"granted" and reason:null; showing a denied state here misdocuments what administrators and SDK hosts will observe.

{"action":"status","result":"status","storedState":"granted","effectiveState":"denied","reason":"policy-blocked","policy":"blocked","needsPrompt":false,"prompt":null,"challenge":null}

sdk/dotnet/Microsoft.Mxc.Sdk/TelemetryConsentState.cs:26

• The guidance names MxcTelemetry.GetStatus().NeedsPrompt, but neither GetStatus nor a NeedsPrompt property exists in this API; the available policy-aware query is MxcTelemetry.NeedsConsentPrompt(). As written, consumers cannot follow the documented recommendation.

    /// callers must use <c>MxcTelemetry.GetStatus().NeedsPrompt</c> or
    /// <c>MxcTelemetry.NeedsConsentPrompt()</c>, not this state alone, to decide

sdk/node/src/sandbox.ts:414

• SandboxSpawnOptions is also the options type accepted by the one-shot spawnSandbox, spawnSandboxAsync, and spawnSandboxFromConfig APIs, but those paths never copy options.telemetry into the execution config. A caller can therefore pass this valid property to a one-shot API and have it silently ignored. Either apply this option in the shared one-shot spawn path or use a state-aware-only options type so the unsupported combination is rejected by TypeScript.

src/core/mxc_engine/src/lib.rs:171

• A timed-out one-shot SDK run is currently emitted as a successful execution: ScriptResponse::default() leaves exit_code at 0, and emit_completion_event determines success solely from exit_code != 0. Set the timeout exit code to -1, matching Runner::run in wxc_common/src/sandbox_process.rs:435-440, so the event carries the Timeout failure reason.
  src/core/mxc_engine/src/lib.rs:176
• An OS-level wait failure has the same misclassification: the default exit code is 0, so one-shot SDK telemetry records outcome="success" and suppresses the Error event even though error_message and PostLaunchFailed are set. Use a nonzero synthetic exit code for this error response.
  sdk/dotnet/Microsoft.Mxc.Sdk/Microsoft.Mxc.Sdk.csproj:60
• This only guarantees the matching native profile for Release. If NativeMethods.g.cs is already up to date, GenerateNativeBindings can skip its Cargo command; a subsequent Debug build then has no target that ensures target/debug/mxc_ffi exists and the resolver can fall back to the release library. That breaks the documented Debug test setup (including the debug-only consent-store override). Run this ensure target for both configurations using $(MxcCargoProfileArg).

  <Target Name="EnsureNativeLibraryForConfiguration"
          AfterTargets="GenerateNativeBindings"
          Condition="'$(Configuration)' == 'Release'">
    <Exec Command="cargo build -p mxc_ffi --features dotnetsdk --release" WorkingDirectory="$(MxcSrcDir)" />

docs/telemetry/telemetry-policy.md:155

• This formula describes a build-time telemetry switch, but the implemented gate is per-invocation and also requires provider availability. As written, the admin documentation contradicts the four-term contract in telemetry-consent-design.md and can imply that a telemetry-capable build collects without telemetry.enabled: true. Use the same four terms here.

collect = policy_permits AND user_consented AND build_has_telemetry_enabled

src/core/wxc_common/src/telemetry/events.rs:66

• The event name is still MXC.Execution; only this documentation dropped the MXC. prefix. That conflicts with the preserved collector contract and with docs/telemetry/telemetry.md, which identifies MXC.Execution as the event name. Keep the exact identity in this public event documentation.